fix(arcup): a release is newer than its own pre-release - #300
Conversation
version_gt strips the pre-release tag from both arguments before comparing,
so 0.3.0 and 0.3.0-rc.1 reduce to the same numbers, fall through every
comparison and reach the final `return 1`. SemVer orders a release above any
pre-release of the same version, so the answer should be true.
The effect is on anyone running a pre-release of the installer.
check_installer_up_to_date never tells them the release shipped, and
update_arcup refuses to move:
if ! version_gt "$remote_version" "$ARCUP_INSTALLER_VERSION"; then
so `arcup --self-update` from 0.3.0-rc.1 to 0.3.0 reports it is already
current. The existing tests cover the two cases that already worked and not
this one.
Keep the pre-release tags aside and, when major.minor.patch are equal, treat
an empty tag as the higher precedence. Ordering two pre-releases of the same
version is left alone, since no caller compares them.
Installer version bumped per the note at the top of the script.
romac
added
pending-import
|
The red Same test, same signal: which matches the report in #298 that the job is red on unrelated branches including This PR touches Worth noting alongside #248 — the arcup shell suite is not run in CI, so the two cases added here would not be exercised there either until that lands. |
|
Confirming the CI attribution from the #298 side (I filed the job-level history analysis there): the only failing check on this head is I also ran the parts your Windows checkout couldn't. On Linux at head Verification of the fix itself, from sourcing
And confirmed on the #248 point: no workflow references |
|
@JspIIV Thanks! Can you please merge latest |
…e-beats-prerelease
|
Done — Re-ran the shell suite on the merged head: The last two are the cases this PR adds; the first two are the ones that already passed. |
|
The direction is right for #205's headline case ( After cores match, the new branch only special-cases "stable vs any prerelease". Two prereleases of the same core still fall through to version_gt "0.3.0-rc.2" "0.3.0-rc.1" # still false — #205 explicitly expects true
version_gt "1.0.0-beta.11" "1.0.0-beta.2" # still false#205's expected behaviour and checklist both require SemVer §11 identifier ordering, not only "release beats its own RC". Also overlaps with #212, which already implements full §11 precedence (build-metadata strip, numeric vs alphanumeric identifiers, dedicated The |
|
@kutluhaneth46 Your behavioural claim is correct — I executed all three implementations rather than reading them, and the gap is exactly where you say it is:
And #205 does ask for those rows explicitly — both its "Example expected comparisons" block and its Tests checklist list Three things I'd check before acting on the close #300 in favour of #212 part, though. 1. #212 does not carry the version bumpYou correctly flagged the Closing #300 for #212 as it stands would land the more complete algorithm behind a version number that self-update can't act on — the exact failure the bump exists to prevent. #212 needs the bump added before it substitutes for #300. 2. The two PRs conflict, so "if this lands first" is a rebase questionA real trial merge of #300 into #212 gives 3. Relative state of the two PRs#212 was last touched 2026-07-27 (head On reachability — correcting myselfEarlier in this thread I wrote that full identifier ordering "has no caller today." That was right by accident, and the reasoning behind it deserves tightening, because it bears on how urgent your point is. Both call sites compare ARCUP_BIN_URL="https://raw.githubusercontent.com/circlefin/arc-node/main/arcup/arcup"That's raw But that's a property of convention, not of construction. The accepted version regex explicitly permits prerelease suffixes ( Suggested pathRather than closing #300, the lowest-friction resolution is probably for it to expand to full §11 — @JspIIV already offered exactly that ("Happy to add it if you would rather have it complete"), it already has the bump and maintainer momentum, and #212's implementation is available to borrow from with credit. The alternative — land #212 plus the version bump and close #300 — is equally fine, but it's strictly more work than it looks, not less. One CI wrinkle either wayPer #248, no workflow under All version comparisons above were executed on Linux against each branch's actual |
4 participants
What
version_gtdrops the pre-release tag from both arguments before it compares anything:So
0.3.0and0.3.0-rc.1both reduce to0.3.0, every major/minor/patch comparison falls through, and the function reaches its finalreturn 1. SemVer §11 puts a release above any pre-release of the same version, so this one should be true.Measured against the current script:
version_gt 0.3.1-rc.1 0.3.0version_gt 0.3.0-rc.1 0.3.0version_gt 0.3.0 0.3.0-rc.1version_gt 0.3.0 0.3.0-rc.2Why it matters
It affects anyone running a pre-release of the installer itself.
check_installer_up_to_datenever prints the "outdated" warning once the release ships, andupdate_arcuprefuses to move:so
arcup --self-updatefrom0.3.0-rc.1to0.3.0reports that it is already current. There is no way out of an rc build except reinstalling by hand.test_version_comparisoncovers the two cases that already worked and not this one, which is why it went unnoticed.Change
Keep the pre-release tags aside instead of discarding them, and when major/minor/patch are equal treat an empty tag as the higher precedence.
Ordering two pre-releases of the same version (
rc.2vsrc.10) is deliberately left alone — it needs the full SemVer identifier comparison and no caller does it, since arcup only ever compares against its own version. Happy to add it if you would rather have it complete.ARCUP_INSTALLER_VERSIONbumped to0.2.1, per the note at the top of the script.Tests
Two cases added to
test_version_comparison. Againstmainthe first one fails:With the change,
bash arcup/test_arcup.shpasses through toarchive path traversal fails.One note on running the suite locally:
test_archive_link_entries_failfails on a Windows checkout becauseln -sneeds privileges there. It fails the same way on an untouched tree, so it is unrelated to this change — I could not exercise that case or the two after it.Closes #205.